feat(trace): support runtime sampler updates - #8716
Conversation
|
|
Pull request dashboard statusClosed · refreshed 2026-08-18 03:16 UTC Status above doesn't look right?
|
There was a problem hiding this comment.
Pull request overview
This PR adds support for updating the SdkTracerProvider sampler at runtime so that new spans (including those created from previously obtained Tracer instances) observe the updated sampler.
Changes:
- Introduces
SdkTracerProvider.setSampler(Sampler)and corresponding test coverage for runtime updates and null rejection. - Makes the shared sampler reference mutable (
volatile) so new span creation uses the latest sampler. - Adds a test verifying sampler updates affect spans started from an existing
Tracer.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SdkTracerProvider.java | Adds public runtime sampler setter and keeps sampler accessible via getSampler(). |
| sdk/trace/src/main/java/io/opentelemetry/sdk/trace/TracerSharedState.java | Makes sampler mutable to allow runtime updates for span creation. |
| sdk/trace/src/test/java/io/opentelemetry/sdk/trace/SdkTracerProviderTest.java | Adds tests for sampler updates, null rejection, and existing tracer behavior. |
Suppressed comments (1)
sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SdkTracerProvider.java:125
getSampler()now returns the current sampler (it can change at runtime), but its Javadoc still says "configured" which reads like an immutable build-time value.
/** Returns the configured {@link Sampler}. */
public Sampler getSampler() {
return sharedState.getSampler();
}
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| void setSampler(Sampler sampler) { | ||
| this.sampler = sampler; | ||
| } |
| /** | ||
| * Updates the sampler used for new spans. | ||
| * | ||
| * @param sampler the sampler to use for sampling new spans. | ||
| */ |
There was a problem hiding this comment.
Follow-up: added sampler lifecycle handling so replaced samplers are shut down, and samplers set after provider shutdown are immediately shut down. Added tests covering both cases. :sdk:trace:test passes.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
I am pretty sure that this is NOT desired behavior. I don't think users generally want a given tracer's sampler (or in this case, all tracers from a tracer provider) to be swapped out at any time at runtime. We try and keep the core of the sdk largely immutable, and this is a design principle.
Imagine that you have production user code that sets a sampler, and then some library code comes along and changes it to something entirely different, long after the sdk is created. That just makes things completely difficult to reason about or troubleshoot when the expected thing eventually happens.
Can you elaborate on what your use case is? Is it something that you can accomplish by building a mutable sampler instead for your purposes, rather than mutating the tracer provider?
|
hanks for the feedback. I don't have a concrete use case that requires replacing the sampler on SdkTracerProvider after construction. The intent of this PR was to provide runtime sampler configuration, but I agree that mutating the provider's sampler conflicts with the SDK's immutability design and can make behavior difficult to reason about. A mutable Sampler would be a better fit for cases that require changing sampling behavior at runtime. I'll close this PR rather than introduce an API that isn't justified by a concrete use case. |
Summary
Add runtime sampler updates to
SdkTracerProviderso a sampler can be changed after the provider is built.Changes
SdkTracerProvider.setSampler(Sampler)Verification
./gradlew :sdk:trace:test --tests io.opentelemetry.sdk.trace.SdkTracerProviderTest./gradlew :sdk:trace:test./gradlew spotlessApplygit diff --check